Don't show key icon for empty passwords #890
Conversation
There was a problem hiding this comment.
Just did a first pass as this is still in draft @graeme. Left a couple of comments. I can have another look when it’s ready with a PR description etc.
| } | ||
| } | ||
|
|
||
| private func decryptCredentials(_ credentials: SecureVaultModels.WebsiteCredentials) throws -> SecureVaultModels.WebsiteCredentials { |
There was a problem hiding this comment.
Maybe move this down to the // MARK: Private section
| from: vault, | ||
| or: passwordManager, | ||
| withPartialMatches: includePartialAccountMatches) { [weak self] accounts, error in | ||
| getCredentials(forDomain: domain, from: vault, or: passwordManager) { [weak self] credentials, error in |
There was a problem hiding this comment.
@graeme there is a regression here since withPartialMatches: includePartialAccountMatches has not been implemented when replacing getAccounts with getCredentials which is what allows subdomain matching to work e.g.
- if you have credentials saved for
nytimes.com - you visit the website nytimes.com
- when you click to login you are redirected to
myaccount.nytimes.com - your
nytimes.comcredentials should be a suggestion since it’s the same TLD (nytimes.com)
|
@graeme unfortunately I think that last change impacted the |
| @@ -1,7 +1,7 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <Scheme | |||
| LastUpgradeVersion = "1540" | |||
| version = "1.7"> | |||
| version = "1.8"> | |||
There was a problem hiding this comment.
It might worth checking with the team before upgrading this (and SubscriptionTests.xcscheme below)
There was a problem hiding this comment.
Ah I'll revert before merging as this isn't really the place.
* main: Moves evaluateJS methods to main thread (#908) Bumped C-S-S to 6.4.0 (#910) Don't show key icon for empty passwords (#890) Hotfix: fix index out-of-bounds in startAttachingCrashLogMessages (#906) Add support for additionalCampaignPixelParams (#899) Update breakage report locale to JSON format (#905) Add support for additionalCampaignPixelParams
Please review the release process for BrowserServicesKit here.
Required:
Task/Issue URL: https://app.asana.com/0/1202926619870900/1207697076491402/f
iOS PR: duckduckgo/iOS#3070
macOS PR: duckduckgo/macos-browser#2962
What kind of version bump will this require?: Patch
Optional:
Tech Design URL: https://app.asana.com/0/481882893211075/1207750849382981
Description:
There has been an issue around for some time where the autofill key icon (indicating that there is autofill data for that field) was showing in the password field despite there being no password stored for a domain. This was because we were using the existence of any stored data for that domain as the source of truth for showing the icon in both username and password fields.
This PR fixes that bug by adding the ability to fetch the passwords themselves and check if they are non-empty.
Steps to test this PR:
See platform PRs
OS Testing:
Internal references:
Software Engineering Expectations
Technical Design Template